feat: add gpt-5.1-codex-max support with xhigh reasoning and persistent logging#27
feat: add gpt-5.1-codex-max support with xhigh reasoning and persistent logging#27riatzukiza merged 23 commits intostagingfrom
Conversation
- Update overview to reflect new gpt-5.1-codex-max model as default - Add note about xhigh reasoning effort exclusivity to gpt-5.1-codex-max - Document expanded model lineup matching Codex CLI
- Document new Codex Max support with xhigh reasoning - Note configuration changes and sample updates - Record automatic reasoning effort downgrade fix for compatibility
- Add gpt-5.1-codex-max configuration with xhigh reasoning support - Update model count from 20 to 21 variants - Expand model comparison table with Codex Max as flagship default - Add note about xhigh reasoning exclusivity and auto-downgrade behavior
- Add flagship Codex Max model with 400k context and 128k output limits - Configure with medium reasoning effort as default - Include encrypted_content for stateless operation - Set store: false for ChatGPT backend compatibility
- Change default model from gpt-5.1-codex to gpt-5.1-codex-max - Align minimal config with new flagship Codex Max model - Provide users with best-in-class default experience
- Add gpt-5.1-codex-max example configuration - Document xhigh reasoning effort exclusivity and auto-clamping - Remove outdated duplicate key example section - Clean up reasoning effort notes with new xhigh behavior
- Document new per-request JSON logging and rolling log files - Note environment variables for enabling live console output - Help developers debug with comprehensive logging capabilities
- Add rolling log file under ~/.opencode/logs/codex-plugin/ - Write structured JSON entries with timestamps for all log levels - Maintain per-request stage files for detailed debugging - Improve error handling and log forwarding to OpenCode app - Separate console logging controls from file logging
- Add model normalization for all codex-max variants - Implement xhigh reasoning effort with auto-downgrade for non-max models - Add Codex Max specific reasoning effort validation and normalization - Ensure compatibility with existing model configurations
- Add xhigh to ConfigOptions.reasoningEffort union type - Add xhigh to ReasoningConfig.effort union type - Enable type-safe usage of extra high reasoning for gpt-5.1-codex-max
- Add test case for new flagship Codex Max model - Verify medium reasoning effort with auto summary and medium verbosity - Ensure comprehensive testing coverage for all model variants
- Add default Authorization header to createCodexHeaders mock - Prevent test failures due to missing required headers - Ensure consistent test environment across all test runs
- Add tests for rolling log file functionality - Update test structure to handle module caching properly - Test console logging behavior with environment variables - Verify error handling for file write failures - Ensure appendFileSync is called for all log entries
- Add missing appendFileSync mock to prevent test failures - Ensure all file system operations are properly mocked - Maintain test isolation and consistency
- Add appendFileSync mock to prevent test failures from logger changes - Clear all mocks properly in beforeEach setup - Ensure test isolation and consistency across test runs
- Add existsSync, appendFileSync, writeFileSync, mkdirSync mocks - Clear all mocks in beforeEach for proper test isolation - Prevent test failures from logger persistent logging changes - Ensure consistent test environment across all test files
- Add model normalization tests for all codex-max variants - Test xhigh reasoning effort behavior for codex-max vs other models - Verify reasoning effort downgrade logic (minimal/none → low, xhigh → high) - Add integration tests for transformRequestBody with xhigh reasoning - Ensure complete test coverage for new Codex Max functionality
…gging - Add comprehensive spec for Codex Max integration with xhigh reasoning - Document persistent logging requirements and implementation plan - Track requirements, references, and change logs for both features
|
Caution Review failedThe pull request is closed. Summary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThe PR introduces an asynchronous rolling log system replacing synchronous file I/O, adds Codex Max model variant support with xhigh reasoning effort granularity, and updates test infrastructure to support the new logging mechanics and model handling. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application Code
participant LogQueue as Log Queue
participant FileIO as Async File I/O
participant Console as Console
participant OpencodeClient as OpencodeClient
App->>LogQueue: enqueueLog(entry)
LogQueue->>LogQueue: Check queue size
alt Queue Full
LogQueue->>Console: Warn queue overflow
LogQueue->>LogQueue: Drop oldest entry
else Queue OK
LogQueue->>LogQueue: Add entry
end
App->>FileIO: triggerFlush() or size limit reached
FileIO->>FileIO: maybeRotate()
alt Rotation Needed
FileIO->>FileIO: rotateLogs() - rename, remove old
end
FileIO->>FileIO: appendFile() to rolling log
FileIO->>OpencodeClient: emit(entry) if available
alt OpencodeClient unavailable
FileIO->>Console: Log failure
end
App->>FileIO: flushRollingLogsForTest()
FileIO->>FileIO: Drain queue
FileIO->>FileIO: Rotate if needed
FileIO-->>App: Promise resolved
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes
Areas requiring extra attention:
Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (2)
Comment |
- Combine persistent logging changes with test environment detection - Keep our versions of test files with comprehensive logging coverage - Integrate staging changes while preserving Codex Max functionality
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (10)
AGENTS.mdis excluded by none and included by noneCHANGELOG.mdis excluded by none and included by noneREADME.mdis excluded by none and included by noneconfig/full-opencode.jsonis excluded by none and included by noneconfig/minimal-opencode.jsonis excluded by none and included by nonedocs/development/CONFIG_FIELDS.mdis excluded by none and included by nonedocs/development/TESTING.mdis excluded by none and included by nonescripts/test-all-models.shis excluded by none and included by nonespec/gpt-51-codex-max.mdis excluded by none and included by nonespec/persistent-logging.mdis excluded by none and included by none
📒 Files selected for processing (9)
lib/logger.ts(3 hunks)lib/request/request-transformer.ts(4 hunks)lib/types.ts(2 hunks)test/codex-fetcher.test.ts(1 hunks)test/logger.test.ts(1 hunks)test/plugin-config.test.ts(1 hunks)test/prompts-codex.test.ts(3 hunks)test/prompts-opencode-codex.test.ts(2 hunks)test/request-transformer.test.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
test/prompts-codex.test.ts (1)
lib/cache/session-cache.ts (1)
codexInstructionsCache(76-76)
test/request-transformer.test.ts (2)
lib/request/request-transformer.ts (3)
normalizeModel(307-344)getReasoningConfig(376-444)transformRequestBody(839-974)lib/types.ts (2)
RequestBody(131-153)UserConfig(24-31)
test/logger.test.ts (1)
lib/logger.ts (5)
LOGGING_ENABLED(7-7)logRequest(58-74)logDebug(76-78)logWarn(84-86)logInfo(80-82)
lib/logger.ts (2)
lib/utils/file-system-utils.ts (1)
getOpenCodePath(22-24)lib/constants.ts (1)
PLUGIN_NAME(7-7)
🔇 Additional comments (23)
lib/logger.ts (5)
8-10: LGTM: Debug flag logic updated correctly.The debug flag logic now enables debug mode when either
DEBUG_CODEX_PLUGINorENABLE_PLUGIN_REQUEST_LOGGINGis set, and console logging is controlled separately. This provides flexible logging control.
22-28: LGTM: RollingLogEntry structure is well-defined.The
RollingLogEntrytype provides a clean structure for both file persistence and transmission to the client. The optionalextrafield allows for flexible metadata attachment.
80-90: LGTM: New logging functions provide convenient API.The new
logInfo,logWarn, andlogErrorfunctions provide a convenient public API that properly normalizes data and delegates to theemitfunction.
92-117: Verify error handling in client log forwarding.The async log forwarding to
loggerClient.app.log()usesvoidand catches errors, logging them vialogToConsole. This prevents the forwarding from blocking the main flow, but be aware that if the console logging is disabled and the client fails, the log entry will only be persisted to file.This is the intended behavior based on the design, but verify that this matches your requirements:
- File logging always happens (if enabled)
- Client forwarding is best-effort
- Failures fall back to console only if console logging is enabled or level is warn/error
169-178: LGTM: appendRollingLog handles errors gracefully.The function properly creates the log directory, appends entries as newline-delimited JSON, and falls back to console logging on failure.
test/prompts-codex.test.ts (2)
9-9: LGTM: Filesystem mock extended for rolling logs.The test properly extends the filesystem mock to include
appendFileSync, aligning with the new rolling log functionality inlib/logger.ts. The mock is correctly cleared inbeforeEach.Also applies to: 19-20, 25-26, 44-44
48-48: LGTM: Type-safe global fetch assignment.Using
(global as any).fetchresolves TypeScript type constraints while maintaining the test's functionality.test/prompts-opencode-codex.test.ts (1)
13-16: LGTM: Comprehensive filesystem mock setup.The test file properly mocks the
node:fsmodule with both default and named exports, covering all filesystem operations used by the logger. Mocks are correctly reset inbeforeEach.Also applies to: 24-35, 70-73
test/plugin-config.test.ts (1)
12-12: LGTM: Mock updated for consistency.The
appendFileSyncmock is added to maintain consistency with other test files and support the new rolling log functionality.lib/types.ts (1)
37-37: LGTM: Type definitions extended for xhigh reasoning effort.The
xhighoption is properly added to bothConfigOptions.reasoningEffortandReasoningConfig.effortas a literal union type, maintaining type safety.Also applies to: 47-47
test/request-transformer.test.ts (3)
65-70: LGTM: Comprehensive codex-max normalization tests.The tests verify that various codex-max input patterns (
gpt-5.1-codex-max,gpt51-codex-max,gpt-5-codex-max,codex-max) all normalize to the canonicalgpt-5.1-codex-max.
134-158: LGTM: gpt-5.1-codex-max reasoning config tests cover key scenarios.The test suite verifies:
- Default medium effort for codex-max
- xhigh is allowed for codex-max
- minimal/none downgrades to low for codex-max
- xhigh downgrades to high for non-codex-max models
785-813: LGTM: Integration tests validate xhigh handling in transformation.The tests verify that
transformRequestBodycorrectly preserves xhigh for codex-max and downgrades it to high for other models, ensuring the reasoning configuration is properly applied end-to-end.test/logger.test.ts (8)
3-15: Mock setup looks correct.The addition of
appendFileSyncto the filesystem mocks properly supports the new rolling log functionality. The mock structure is consistent between thefsMocksobject and thevi.mockregistration.
17-20: Home directory mock is appropriate.Using a fixed
/mock-homepath provides predictable assertions for log file paths throughout the test suite.
22-37: Test setup is thorough and correct.The module-level spy setup combined with
beforeEachcleanup provides proper test isolation. Usingvi.resetModules()is the right approach for testing environment-dependent module state likeLOGGING_ENABLED.
50-69: Test coverage for logRequest is comprehensive.This test properly verifies that
logRequestwrites both a stage-specific JSON file and appends to the rolling log. The assertions check paths, encoding, payload structure, and the absence of console output.Minor: The test name "by default" could be more specific since it explicitly mocks
existsSyncto returnfalse, but this is a nitpick.
71-79: logDebug test correctly verifies silent rolling log behavior.The test confirms that
logDebugappends to the rolling log without console output when logging is not explicitly enabled, which matches the expected behavior.
81-88: logWarn test validates console output behavior.Correctly verifies that warnings are emitted to console even without environment variable overrides, which is appropriate for the warn level.
90-102: logInfo test properly validates environment-dependent console mirroring.The test correctly uses
vi.resetModules()to reload the logger module with different environment state. This is the right pattern for testing theLOGGING_ENABLEDconstant behavior.
104-115: Persistence failure test validates error resilience.This test properly verifies that when stage file persistence fails, the system logs a warning and continues with the rolling log append. This ensures partial failures don't completely break logging.
lib/request/request-transformer.ts (2)
307-344: Codex‑max normalization order and heuristics look correctThe
hasCodexMaxdetection plus earlyif (hasCodexMax)branch ensures all commoncodex-max/codexmaxvariants (with dots, spaces, underscores, or suffixes) are canonicalized togpt-5.1-codex-maxbefore the generic codex branches run, without changing behavior for existing codex/codex‑mini mappings. This segment looks sound and backward‑compatible.
376-444: xhigh reasoning effort handling for Codex Max is consistent and containedThe new
isCodexMaxflag andrequestedXHighgate cleanly enforce thatxhighonly survives for Codex Max, while all other models transparently downgradexhightohigh. The Codex‑max branch’s clamping ofminimal/nonetolowpreserves the prior “never below low for codex family” behavior, and other codex / non‑codex branches remain unchanged. I don’t see any logic gaps or regressions here.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/prompts-codex.test.ts (1)
52-56: The review concern is valid—fix the test isolation violation.Verification confirms that
delete (global as any).fetchpermanently removes the global fetch, breaking test isolation. While other tests in the suite set fetch fresh in theirbeforeEach(so they won't immediately fail), this approach violates proper cleanup patterns and can cause issues with test execution order or when tests run in different contexts.The codebase already demonstrates the better pattern:
test/auth.test.tspreserves and restoresconsole.errorusing the same technique suggested here. Apply the fix as indicated in the review comment.lib/request/request-transformer.ts (1)
290-356: Fix:xhighis not a valid reasoning effort level per official Codex docsBlocker found: The code accepts and may return
"xhigh"for Codex Max models, but official Codex CLI config and OpenAI/Azure reasoning guidance specify effort values as "minimal", "low", "medium" (default), or "high" only.The
xhighcheck downgrade non-Codex Max tohigh, but Codex Max can still receive and returnxhighunchecked (lines 309-311 and return statement). Either:
- Remove the
xhighbranch entirely and reject/normalize it tohighfor all models, or- Confirm
xhighis a valid internal value and normalize it tohighbefore returning.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
README.mdis excluded by none and included by none
📒 Files selected for processing (6)
lib/logger.ts(4 hunks)lib/request/request-transformer.ts(4 hunks)lib/types.ts(2 hunks)test/codex-fetcher.test.ts(1 hunks)test/plugin-config.test.ts(1 hunks)test/prompts-codex.test.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
test/prompts-codex.test.ts (1)
lib/cache/session-cache.ts (1)
codexInstructionsCache(76-76)
lib/logger.ts (2)
lib/utils/file-system-utils.ts (1)
getOpenCodePath(22-24)lib/constants.ts (1)
PLUGIN_NAME(7-7)
🪛 GitHub Actions: CI
lib/logger.ts
[error] 126-126: TS2367: This comparison appears to be unintentional because the types '"debug" | "info"' and '"error"' have no overlap.
🪛 GitHub Check: Lint & Typecheck
lib/logger.ts
[failure] 126-126:
This comparison appears to be unintentional because the types '"debug" | "info"' and '"error"' have no overlap.
🔇 Additional comments (5)
test/codex-fetcher.test.ts (1)
55-65: Re-seedingcreateCodexHeadersMockafter reset keeps tests stableRe‑applying the
Headers-returning implementation aftermockReset()ensures every test still sees a concreteHeadersinstance even if previous tests adjusted the mock. No blockers here; this setup is robust for the Codex fetch flow tests.test/prompts-codex.test.ts (1)
5-27: FS and fetch mocks look consistent with new logging behaviorThe new
appendFileSyncmock and its export wiring mirror the existing fs mocks and give you a clean hook for rolling-log writes. Clearing it (and the other fs mocks) plus explicitly assigning(global as any).fetchinbeforeEachkeeps state isolated per test. No issues here.Also applies to: 39-45, 48-50
test/plugin-config.test.ts (1)
8-14: Extending thenode:fsmock withappendFileSyncis correctAdding
appendFileSyncto the mockednode:fsexport aligns this test harness with code paths that append to files (e.g., logging). It avoids unexpected “undefined is not a function” failures without changing existing expectations.lib/types.ts (1)
52-65: Blocker: Public documentation cannot be verified in this environmentThe
xhighaddition is properly implemented inlib/types.ts(lines 53, 63) and tested intest/request-transformer.test.tswith correct model-specific behavior (preserved forgpt-5.1-codex-max, downgraded for others). However, I cannot access this repository's public documentation or configuration schemas to confirm that"xhigh"is now documented as a validreasoningEffortvalue.You must verify that any external docs, configuration schemas, or API documentation this project publishes have been updated to include
"xhigh"as a valid value alongside the code changes.lib/request/request-transformer.ts (1)
221-258: Codex Max model normalization is correct and well-testedTests confirm all expected variants normalize properly to the canonical
"gpt-5.1-codex-max": including'gpt-5.1-codex-max','gpt51-codex-max','gpt-5-codex-max', and'codex-max'. ThehasCodexMaxheuristic correctly detects both hyphenated (codex-max) and compact (codexmax) forms, and early return prevents interference with mini/family branches. Reasoning config tests also confirm Codex Max behavior (defaults to medium effort, allows xhigh, downgrades minimal/none to low).
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
lib/logger.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
lib/logger.ts (2)
lib/utils/file-system-utils.ts (1)
getOpenCodePath(22-24)lib/constants.ts (1)
PLUGIN_NAME(7-7)
🔇 Additional comments (3)
lib/logger.ts (3)
80-90: Clean public API addition.The new
logInfo,logWarn, andlogErrorfunctions provide a well-structured logging interface that routes through the unified emit path.
92-117: Well-structured unified emit path.The refactored emit function provides a clean separation of concerns: persist to rolling log, forward to remote client (with fallback), and console output. The error handling appropriately uses
logToConsoledirectly to avoid recursion.
125-128: Past TypeScript error successfully resolved.The redundant type guard
level !== "error"has been removed from line 126, resolving the TS2367 compilation error flagged in the previous review. The current logic correctly narrows the type and handles test environment filtering appropriately.
1 participant
Summary
gpt-5.1-codex-maxas new default model with exclusivexhighreasoning effortKey Changes
codex-maxvariants to canonicalgpt-5.1-codex-maxxhighreasoning exclusive to Codex Max, auto-downgrade for other models~/.opencode/logs/codex-plugin/with optional console mirroringFiles Modified
lib/request/request-transformer.ts,lib/types.ts,lib/logger.tsconfig/full-opencode.json,config/minimal-opencode.jsonAGENTS.md,README.md,CONFIG_FIELDS.md,TESTING.mdscripts/test-all-models.shTesting
xhighreasoningBreaking Changes
gpt-5.1-codextogpt-5.1-codex-maxxhighreasoning effort type added to TypeScript interfaces